🗄️ Own WorkflowRun connections and complete schema v1 - #391
Conversation
PR #391: 🗄️ Own WorkflowRun connections and complete schema v115 files, +1345 / -287 Scope🔴 PR has 1632 lines changed. Split into focused PRs. 🟡 1632 lines changed. PRs under 400 receive more thorough review. Structural✅ No structural bloat detected. Slop✅ Slop indicators look low. Static Analysis✅ Oxlint found no issues. CorrectnessNo extraneous code patterns detected. |
fe00594 to
02a7363
Compare
System/software architecture reviewVerdict: APPROVE as stack layer 2. I found no architectural blocker in #391. This is a coherent continuation of #389 and the correct ownership boundary for the rest of #365:
The three version domains are also kept distinct: XMD schema v1, DOFS schema v5, and Workspace-root format v1. Completing the unreleased schema in place is consistent with the pre-release decision; adding a migration or schema v2 here would create a compatibility contract we do not need. The canonical-empty validation is appropriately narrow for this stack layer. It prevents #391 from claiming or accepting nonempty state before the root implementation can prove it. The next root layer must replace that narrow recognizer with full semantic validation before any mutation can commit a nonempty root. Constraints for the dependent layers:
The code and specification changes agree on the current boundary: this PR owns and recognizes the canonical empty Workspace, but does not expose mutation, publish nonempty roots, or claim the atomic effect coordinator. That makes #391 independently mergeable without prematurely weakening the later durability proof. |
Depends on #389.
Stack layer 2 for #365, replacing the authoritative-connection and complete-schema portion of draft #387. This PR does not close #365.
Why
WorkflowRun handles need one provider-owned SQLite/DOFS connection boundary per canonical run path before retained Workspace mutations can be published atomically. The unreleased version-1 database also needs its final complete structure now, without introducing a migration from the unsupported intermediate pre-release shape.
What changes
Before:
Each handle owned a SQLite connection, and schema version 1 contained only workflow metadata and the filtered journal.
After:
Each canonical run path resolves to one provider-owned entry containing the physical SQLite connection, cooperative queue, pinned DOFS wrapper, WorkspaceFilesystem, and synchronous savepoint allocator. Handles are scope-owned leases. Fresh databases atomically receive the complete frozen version-1 schema, canonical content-addressed empty Workspace root, current-root pointer, and journal root association. Existing databases are recognition-only and strict corruption failures remain unchanged on disk.
Foreign-key failures are interpreted as invalid stop reasons only at the two statements that write stop-reason event references. Other Workspace, root, manifest, blob, and journal-root foreign-key failures remain storage failures and roll back their transactions. An XMD-identified database with
user_version = 0is partial initialization and is rejected as corruption without changing the file.How it works
Review guide
Start with:
packages/workflow/src/deno/connections.tsThen review:
packages/workflow/src/deno/provider.tsfor provider lifetime and atomic initializationpackages/workflow/src/deno/schema.tsfor the frozen structural manifest and strict recognitionpackages/workflow/src/deno/workspace/empty.tsfor root-format-1 identity and semantic validationpackages/workflow/tests/workflow-run-storage.test.tsandworkflow-run-journal.test.tsLook carefully at:
BEGIN IMMEDIATEWhat must stay true
WorkflowRunDatabase.transact()remains the only top-level transaction owner; DOFS initialization nests through uniquely named SQLite savepoints.How to verify it
deno task setup— passed.deno task lint— passed with 0 errors.deno task check— passed.deno task test— passed: 401 tests, 2,744 steps, 0 failures.deno task check:jsr— passed (Success Dry run complete).git diff --check origin/agent/issue-365-1-vendor-dofs...HEAD— passed.deno task verify— passed all nine applicable commands under the repository CI Node 22 runtime; the tracked tree remained unchanged.Scope
Included
Intentionally unchanged
<File>, lifecycle, Worker Shell, Git, FUSE, subprocess, workerd, or garbage collection behaviorNew abstractions
WorkflowRunConnectionsowns one authoritative adapter entry per canonical run path.SavepointManageradapts synchronous DOFS transactions to the caller-owned SQLite transaction.Risks and limitations
Scope confirmation